Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Delete netkan tmp files if cache fills up #3169

Merged
merged 1 commit into from
Oct 13, 2020
Merged

Delete netkan tmp files if cache fills up #3169

merged 1 commit into from
Oct 13, 2020

Conversation

HebaruSan
Copy link
Member

Problem

The Inflator's /tmp partition filled up:

netkan@945a9d40d33e:/tmp/CdFileMgr$ du -hs *
1.6G    136049aa-d02d-4a.tmp
1.6G    190528d6-ffe7-44.tmp
1.6G    197cb16c-7bd8-49.tmp
91M    241da6c0-ee1f-45.tmp
91M    283785f4-c7a7-4e.tmp
91M    28df1ab1-cafd-4b.tmp
1.6G    2fabe926-dd25-46.tmp
91M    322b5b76-38b6-46.tmp
91M    3892e70d-d10d-41.tmp
1.6G    3d4376f1-748f-4f.tmp
1.6G    45782341-d73b-4c.tmp
91M    4c1f0f27-b592-4c.tmp
1.6G    5803da08-e7fb-4b.tmp
91M    610da03d-dba6-49.tmp
91M    67901f4e-6360-4a.tmp
91M    9e59cf92-ea96-48.tmp
91M    b8ec7a18-6db6-4e.tmp
1.6G    bcd5650b-fc6c-47.tmp
1.6G    c3fceeff-78d4-48.tmp
91M    c44a1bbb-b313-42.tmp
1.6G    c68dc340-e109-4b.tmp
1.6G    cc9752a0-ce66-4d.tmp
91M    d34b2f4a-ab1b-49.tmp
91M    e115d711-0f3f-44.tmp
1.6G    e529f468-8441-4c.tmp
1.6G    f0b979c7-b3b7-48.tmp
91M    f5d8bc81-dd90-45.tmp

Cause

The cache filled up first because HumanStuff (see KSP-CKAN/NetKAN#8142) released many enormous versions in rapid sequence (about 12GB in under 30 days). When the Inflator attempted to move a downloaded file into the cache, the attempt failed, and the temp file was not cleaned up. Over time the temp files accumulated until /tmp was full.

Changes

Now if NetFileCache.Store fails, the Inflator deletes the temp file.

(Note that some of HumanStuff's bloat has since been pruned, with the latest download only 366 MB, so with luck we should not have the cache fill up again.)

Fixes #3168.

@HebaruSan HebaruSan added Bug Something is not working as intended Easy This is easy to fix Pull request Netkan Issues affecting the netkan data labels Oct 9, 2020
@HebaruSan HebaruSan requested a review from techman83 October 9, 2020 18:26
@DasSkelett
Copy link
Member

What about moving the error handling inside NetFileCache.Store()? This way the client could also benefit from that behavior. I would limit it to the move ==True case, because in the other one you likely want to keep the source file:

CKAN/Core/Net/NetFileCache.cs

Lines 514 to 517 in 7e575e0

if (move)
{
tx_file.Move(path, targetPath);
}

@HebaruSan
Copy link
Member Author

I thought about that, but it's not really an accepted failure mode of a move-like operation; mv doesn't delete the source file if the destination can't be written. And it isn't guaranteed that the source file for Store will always be a temp download file that we don't care about; maybe it's a file the user downloaded (currently import uses move=false, but it might not always be), and maybe we'll want to do something else with it if Store fails.

In the specific case of Netkan we know the conditions in which the Inflator runs, so we can make the decision to trash the file, so I thought that was the right place for this logic.

techman83
techman83 approved these changes Oct 13, 2020
View reviewed changes
Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't had a lot of reports of problems with the cache and this is definitely a pretty specific problem relating to NetKAN. Looks good to me!

@HebaruSan HebaruSan merged commit 1dc6b0e into KSP-CKAN:master Oct 13, 2020
@HebaruSan HebaruSan deleted the fix/tmp-full branch November 5, 2020 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@techman83 techman83 techman83 approved these changes

Assignees
No one assigned
Labels
Bug Something is not working as intended Easy This is easy to fix Netkan Issues affecting the netkan data
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] NetKAN leaving stale tmp files around
3 participants
@HebaruSan @DasSkelett @techman83